Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update openfisca installation process #384

Merged
merged 8 commits into from
Oct 17, 2016

Conversation

fpagnoux
Copy link
Contributor

@fpagnoux fpagnoux commented Oct 11, 2016

No description provided.

Copy link
Member

@MattiSG MattiSG left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Il y a des instructions à modifier / supprimer ll. 126-128.


```sh
cd mes-aides-ui
pip install -r openfisca/requirements.txt
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ne peut-on pas mettre ça dans le postinstall, ou au moins dans un script NPM non lifecycle ?

@MattiSG
Copy link
Member

MattiSG commented Oct 11, 2016

Manque paster serve openfisca/api_config.ini. Idem, un script non lifecycle serait pas mal, comme pour db :)


```sh
cd mes-aides-ui
pip install -r openfisca/requirements.txt
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--user (ou documenter virtualenv)

@MattiSG
Copy link
Member

MattiSG commented Oct 11, 2016

Je fais pip install -r openfisca/requirements.txt --user --upgrade && paster serve openfisca/api_config.ini.

@MattiSG
Copy link
Member

MattiSG commented Oct 11, 2016

GTM. J'ajouterais bien des scripts NPM.

@fpagnoux fpagnoux force-pushed the edit-readme-openfisca-installation branch 2 times, most recently from 2e9fd5a to 790a540 Compare October 12, 2016 14:02
@fpagnoux fpagnoux force-pushed the edit-readme-openfisca-installation branch from 790a540 to 3810d07 Compare October 12, 2016 14:04
@fpagnoux
Copy link
Contributor Author

Done.

"db-update": "./import-tests.sh"
"db-update": "./import-tests.sh",
"install-openfisca": "pip install --user --upgrade -r openfisca/requirements.txt",
"openfisca":"paster serve openfisca/api_config.ini"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Manque un espace après les deux points ;)

@@ -72,7 +72,9 @@
"test-integration": "test/integration/run-integration-tests.sh",
"predb": "mkdir -p db",
"db": "mongod --dbpath db",
"db-update": "./import-tests.sh"
"db-update": "./import-tests.sh",
"install-openfisca": "pip install --user --upgrade -r openfisca/requirements.txt",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Je mettrais plutôt ça dans preopenfisca, pour que lancer openfisca via NPM soit toujours à jour de l'état courant de l'application.

Copy link
Contributor Author

@fpagnoux fpagnoux Oct 14, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

J'y vois deux inconvénients :

  • Si j'utilise un virtualenv, je ne peux pas utiliser install-openfisca. Lier les deux scripts m'empêcherait aussi d'utiliser npm run openfisca.
  • Si j'ai installé openfisca en mode éditable pour travailler sur une formule, exécuter la commande install-openfisca me réinstallera openfisca à partir du dépôt distant. Lier les deux scripts m'empêche aussi dans ce cas d'utiliser npm run openfisca.

Copy link
Contributor Author

@fpagnoux fpagnoux Oct 14, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On pourrait le mettre ce script en postinstall, mais ça pose la question suivante: "Est-ce que parfois on veut installer mes-aides sans installer openfisca ?"


```sh
cd mes-aides-ui
npm run install-openfisca # ou pip install --upgrade -r openfisca/requirements.txt si vous utilisez un environnement virtuel
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Il faut probablement expliciter la dépendance à pip.

@MattiSG MattiSG assigned fpagnoux and unassigned MattiSG Oct 14, 2016
@MattiSG
Copy link
Member

MattiSG commented Oct 14, 2016

GTM

@MattiSG
Copy link
Member

MattiSG commented Oct 14, 2016 via email

@MattiSG
Copy link
Member

MattiSG commented Oct 14, 2016 via email

@fpagnoux fpagnoux merged commit 30e5a48 into master Oct 17, 2016
@fpagnoux fpagnoux deleted the edit-readme-openfisca-installation branch October 17, 2016 06:28
@Shamzic Shamzic added this to the Passé milestone Dec 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants